-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Jq rule composition - workflow for async getters #642
Conversation
7aedbab
to
e991606
Compare
…ource to pass in input params in workflow
"alias": "workflow", | ||
"endpoint": "workflow", | ||
"sources": [{ | ||
"method": "static.rle" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is a typo here for static.rle does it still work is the test for invalid rule?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case (the unit test test RuleEngine::load_from_string_literal, which makinly tests that it can take a schema compliant string as input, and produce a rule engine from them), so it does not make any difference (but does look a little confusing now that i'm looking at it)
@@ -197,6 +198,60 @@ impl JsonRpcApiRequest { | |||
} | |||
} | |||
|
|||
#[derive(Clone, Default, Debug)] | |||
pub struct JsonRpcApiError { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this added because JsonRPCAPIResponse doesnt have message:String
Just curious why we need this new entry?
#[derive(Debug, Clone, Serialize, Deserialize)]
pub struct JsonRpcApiResponse {
pub jsonrpc: String,
pub id: Option,
#[serde(skip_serializing_if = "Option::is_none")]
pub result: Option,
#[serde(skip_serializing_if = "Option::is_none")]
pub error: Option,
#[serde(skip_serializing)]
pub method: Option,
#[serde(skip_serializing)]
pub params: Option,
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This (JsonApiError)was added to add stronger typing (as part of my original POC). This will (does) ultimately make code more understandable and enable proper encapsulation, higher type safety and ultimately better maintenance and unit testing in the future.
Minimum allowed line rate is |
What
What does this PR add or remove?
Why
Why are these changes needed?
How
How do these changes achieve the goal?
Test
How has this been tested? How can a reviewer test it?
Checklist